Skip to content

feat(studio): flat inspector foundation + Text group#2120

Open
vanceingalls wants to merge 12 commits into
mainfrom
studio-flat-01-foundation-text
Open

feat(studio): flat inspector foundation + Text group#2120
vanceingalls wants to merge 12 commits into
mainfrom
studio-flat-01-foundation-text

Conversation

@vanceingalls

@vanceingalls vanceingalls commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

What

First PR in a 6-PR stack migrating Studio's right-panel property inspector from an always-expanded stacked-sections layout to a "flat" one-open-at-a-time accordion. This PR lays the foundation: the STUDIO_FLAT_INSPECTOR_ENABLED feature flag, the accordion primitives (FlatRow, FlatSegmentedRow, FlatGroup, PinnedZoneDivider), the flat identity header/footer, and the first migrated group — Text.

Stack: #2120 (this) → #2121 (Style) → #2122 (Layout+Motion) → #2123 (Media) → #2124 (Grade) → #2125 (Pinning + multi-field Text).

Why

The legacy inspector renders every applicable section expanded at once, which gets unwieldy as an element accumulates properties across style/layout/motion/media/grade. The flat redesign shows one section at a time (plus pinned sections), matching a design handoff mock.

How

  • FlatGroup owns the one-open accordion state (openGroupId/onToggleOpen) and pin affordance (onTogglePin), styled per the design mock.
  • FlatTextSection is the first migrated group and the reference implementation every later group's task followed for the isOpen/onToggleOpen/onTogglePin/summary wiring pattern.
  • Includes a same-PR bugfix (found via live browser testing, not caught by any automated test): the Text FlatGroup was rendering unconditionally regardless of element type (empty for non-text elements), and the multi-field fallback doubled the "Text" heading. Fixed by gating on isTextEditableSelection and adding a hideOwnHeading prop to the legacy TextSection fallback.
  • Entirely gated behind STUDIO_FLAT_INSPECTOR_ENABLED (default off) — the legacy panel is untouched and remains the default for all users.

Test plan

  • Every primitive and the Text group have dedicated Vitest suites using real DOM events (click/pointerdown) with exact assertions, not shallow snapshots.
  • Manually verified in Studio via live browser testing against the design mock (this is what caught the bugfix above).
  • Full monorepo test suite green; oxlint/oxfmt clean; this repo's fallow complexity/duplication gate passes.
  • Unit tests added/updated
  • Manual testing performed
  • Documentation updated (not applicable — internal Studio UI behind an off-by-default flag)

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as part of the 15-PR edit-panel redesign stack. Full stack review posted on #2120. No blockers on this PR. — Miga

vanceingalls and others added 3 commits July 10, 2026 18:28
…AT_INSPECTOR_ENABLED

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…eading in multi-field fallback

The flat inspector's Text FlatGroup rendered unconditionally, showing an
empty "Text" header for non-text elements (image, video, etc). Gate it on
isTextEditableSelection(element) so it disappears entirely when there's no
text to edit.

Also, the legacy multi-field TextSection (used as a fallback when an
element has 2+ text fields) rendered its own internal "Text" heading
nested inside the new flat Text FlatGroup, producing a doubled "Text"
heading. Add a hideOwnHeading prop to TextSection (default false, so its
other — legacy, non-flat — call site is unaffected) and pass it from
FlatTextSection's fallback path.

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full Stack Review — Edit Panel Redesign (15 PRs, ~10,700 additions)

Reviewed the entire stack in three tiers: foundation (#2120#2122), domain groups + retirement (#2123#2127), and UX polish (#2128#2190). Per-PR verdicts below; no blockers across the stack.


Tier 1: Foundation (#2120#2122)

#2120 — flat inspector foundation + Text group — Clean. Introduces the shell (header, footer, accordion, value-tier system, primitives) behind STUDIO_FLAT_INSPECTOR_ENABLED. resolveValueTier is a well-designed single decision point. hideOwnHeading flag on TextSection prevents doubled headings without duplication.

#2121 — Style group — Clean. Each sub-block (Fill, Stroke, Radius, Shadow, Blend, Blur, Overflow, Mask, Opacity) owns exactly one CSS domain. Helpers come from propertyPanelHelpers.ts (single source). FlatRadiusRow reuses legacy BorderRadiusEditor for non-uniform corners — good reuse.

#2122 — Layout + Motion groups — Clean. The standout SSOT contribution of the stack: deriveElementTiming extracts the "how do we compute start/duration?" decision into one function consumed by both Layout's keyframe gutter and Motion's timing row. Fixes a real bug where Layout used elDuration ?? 1 while Motion inferred from tweens.

Tier 2: Domain Groups + Retirement (#2123#2127)

#2123 — Media group — Clean. Legacy MediaSection fully replaced, not duplicated. FlatToggle is a genuine new primitive.

#2124 — Grade (color grading) — Nit. The useColorGradingController extraction is the strongest SSOT win in the stack — turns a 200-line inline state machine into a shared hook. During the hybrid window both legacy and flat sections instantiate the hook (two debounce timers, two polling cascades). Resolved by #2126.

#2125 — persisted pinning + multi-field Text — Clean. FlatGroupDescriptor[] data model eliminates per-group copy-paste. usePersistedPinnedGroups is a clean single-path persistence hook.

#2126 — retire legacy Style and Grade — Clean. Pure SSOT cleanup — removes the dual-rendering transition state. The hideFlex escape hatch is gone.

#2127 — parity gaps from legacy-retirement audit — Clean. visibleIntensity helper centralizes the "revive from 0% strength" decision. Radius path simplified by always delegating to BorderRadiusEditor. Stroke width clamping reuses existing normalizePanelPxValue. Keyboard accessibility for compare hold button is thorough.

Tier 3: UX Polish (#2128#2190)

#2128 — sticky accordion headers — Nit. Dual sticky anchoring (top-0 bottom-0) is unconventional but intentional per the design.

#2134 — scroll refactor — Clean. Well-structured three-region layout (before-open, scrollable body, after-open). FlatGroupFlatGroupHeader rename correctly reflects narrowed responsibility.

#2135 — remove section pinning — Clean. Complete disciplined removal — no orphaned props, no dead code.

#2136 — animate expand/collapse — Clean. justToggledIds mechanism handles the Chromium re-trigger edge case thoughtfully. Respects prefers-reduced-motion. Well-commented.

#2142 — widen slider hit area — Clean. 2px → 20px invisible hit area. Minimal, correct fix.

#2186 — slider drag support — Clean. Pointer capture API is the correct modern approach. Properly layered before the debounce PR.

#2190 — debounce slider drag commits — Clean. Draft/debounce pattern gives instant visual feedback while debouncing commits at 40ms. Flush-on-release ensures final value always commits immediately. Broad test updates across sibling sections show thoroughness.


Cross-Stack Architecture

The stack follows a disciplined pattern:

  1. Flag-gated parallel path — flat inspector runs alongside legacy, gated by STUDIO_FLAT_INSPECTOR_ENABLED. No legacy behavior changes.
  2. Suppression flags over removal — when a flat group absorbs a legacy section, the legacy gets hideOwnHeading/hideFlex rather than being removed, enabling safe coexistence. Retired cleanly in #2126.
  3. Helpers carry new knowledge; primitives carry new rendering — every helper justifies itself with a fact the caller doesn't know. No trivial wrappers.
  4. Self-correcting SSOT#2122 identifies and fixes the timing-basis disagreement that would have been an SSOT violation across Layout and Motion. The stack improves as it progresses.

CI: 0 failures across all 15 PRs. Ship the stack.

— Miga

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker on current head 444639d: GitHub reports this PR as CONFLICTING / mergeState DIRTY. Resolve the branch against its stack/base and rerun the required checks before approval. The code review findings are non-blocking, but this head is not mergeable as submitted.

@vanceingalls vanceingalls force-pushed the studio-flat-01-foundation-text branch from 444639d to 6e9d99e Compare July 11, 2026 01:34
@vanceingalls

Copy link
Copy Markdown
Collaborator Author

Rebased the stack base onto latest main (one real conflict in StudioRightPanel.tsx, resolved by merging the DesignPanelPromoteProvider wrapper with the group-selection/hide-all wiring) and force-pushed. Current head 6e9d99ef8 is MERGEABLE, CI is green (32/32). @miguel-heygen — the conflict blocker should be clear now, please re-review when you get a chance.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review after rebase: current head 6e9d99ef8f3aec054aebd05c9b2201f15aec4665 is mergeable, required checks have no failures/pending checks, and prior conflict blocker is resolved. No unresolved review threads; foundation/value-tier and cross-stack contracts remain clean.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseding earlier approval: the latest max-review pass found confirmed correctness issues in this flat-inspector stack (identity/selector-index state, hide-all write races, timing inference, slider pointer/keyboard/reset semantics, and duplicate React keys). Hold merge and require fixes plus re-review on the current stack head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants